[ML][Transforms] remove force flag from _start
#46414
Conversation
|
Pinging @elastic/ml-core |
| ActionListener<StartDataFrameTransformAction.Response> startTaskListener = ActionListener.wrap( | ||
| response -> logger.info("[{}] successfully completed and scheduled task in node operation", transformId), | ||
| failure -> { | ||
| auditor.error(transformId, "Failed to start data frame transform. " + |
There was a problem hiding this comment.
I noticed while troubleshooting, that if we fail on start within the executor, the user could never be the wiser. We should at least audit here.
I will experiment to see if setting the task to failed here is OK.
They will not be able to call _start again as the task already exists, it just won't be flagged as FAILED.
Maybe we want to simply audit and call .stop on the task?
Additionally, this type of failure is present in 7.4. We may want to at least add an audit message there before release.
There was a problem hiding this comment.
There was a problem hiding this comment.
adding an audit for 7.4.1 sounds good to me.
As for "If anything goes wrong within startTask" - I think marking the task as failed and setting a reason is the preferable solution.
| @@ -219,10 +218,17 @@ public void getCheckpointingInfo(DataFrameTransformsCheckpointService transforms | |||
| )); | |||
| } | |||
|
|
|||
| // Here `failOnConflict` is usually true, except when the initial start is called when the task is assigned to the node | |||
| synchronized void start(Long startingCheckpoint, boolean force, boolean failOnConflict, ActionListener<Response> listener) { | |||
There was a problem hiding this comment.
Do I get this right?
Due to this simplification we do not need #46347 anymore and revert it.
There was a problem hiding this comment.
pretty much. Since the ONLY thing that can call start against the task is the node executor, we know that it should be allowed to call start against a started task.
| ActionListener<StartDataFrameTransformAction.Response> startTaskListener = ActionListener.wrap( | ||
| response -> logger.info("[{}] successfully completed and scheduled task in node operation", transformId), | ||
| failure -> { | ||
| auditor.error(transformId, "Failed to start data frame transform. " + |
There was a problem hiding this comment.
adding an audit for 7.4.1 sounds good to me.
As for "If anything goes wrong within startTask" - I think marking the task as failed and setting a reason is the preferable solution.
|
@elasticmachine update branch |
|
@elasticmachine test this |
|
@elasticmachine update branch |
|
run elasticsearch-ci/docs |
…m:benwtrent/elasticsearch into feature/ml-transforms-remove-_start-force
* [ML][Transforms] remove `force` flag from _start * fixing expected error message
* [ML][Transforms] remove `force` flag from _start (#46414) * [ML][Transforms] remove `force` flag from _start * fixing expected error message * adjusting bwc version
The force parameter was removed from start actions in elastic/elasticsearch#46414. This reflects the change in the UI API calls.
The force parameter was removed from start actions in elastic/elasticsearch#46414. This reflects the change in the UI API calls.
The force parameter was removed from start actions in elastic/elasticsearch#46414. This reflects the change in the UI API calls.
It no longer makes sense to include a
forceflag for our_startapi.If a failure occurs, a much safer path is to
_stop?force=trueand then call_startagain. This allows for better clean up of resources and safer run-time guarantees.The flag was not ever added to our docs or the HLRC.